fix(threads): Keep active-turn runtime errors from ending sessions#1261
fix(threads): Keep active-turn runtime errors from ending sessions#1261Snowy7 wants to merge 1 commit intopingdotgg:mainfrom
Conversation
- Treat runtime.error as advisory while an active turn is still running - Add coverage for active-turn runtime.error and no-turn error mapping
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment Tip You can disable poems in the walkthrough.Disable the |
github-actions
bot
added
size:M
There was a problem hiding this comment.
Pull request overview
This PR adjusts provider runtime ingestion so non-terminal runtime.error events received during an active turn no longer force the projected session into an error state, preventing UI/runtime desynchronization while the turn is still streaming/running.
Changes:
- Update
runtime.erroringestion to avoid setting session status toerrorwhen the error corresponds to the currently active turn. - Keep existing terminal handling for
turn.completed(failed),session.state.changed(error), andsession.exited. - Add/adjust tests to cover session projection behavior for
runtime.errorwith and without an active turn.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| apps/server/src/orchestration/Layers/ProviderRuntimeIngestion.ts | Prevents runtime.error from terminally erroring the session while an active turn is still running. |
| apps/server/src/orchestration/Layers/ProviderRuntimeIngestion.test.ts | Adds tests for session projection behavior when runtime.error arrives during an active turn. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const isActiveTurnRuntimeError = | ||
| activeTurnId !== null && (eventTurnId === undefined || sameId(activeTurnId, eventTurnId)); | ||
|
|
||
| // Some provider/runtime errors are advisory while the active turn continues | ||
| // streaming. Keep the projected session runnable until a terminal lifecycle | ||
| // event arrives. | ||
| if (shouldApplyRuntimeError && !isActiveTurnRuntimeError) { |
There was a problem hiding this comment.
The new isActiveTurnRuntimeError logic treats runtime.error events with a missing turnId as belonging to the active turn (and therefore not session-terminal). That branch (eventTurnId === undefined while activeTurnId !== null) isn’t covered by the added tests, and it’s an important case because some adapters can emit runtime errors without a turn id. Add a test that starts a turn, emits runtime.error without turnId, and asserts the session remains running while a runtime.error activity is still recorded.
Snowy7
changed the title
Snowy7
changed the title
Snowy7
changed the title
github-actions
bot
added
size:L
Snowy7
force-pushed
the
feature/runtime-error-session
branch
from
af1c65c to
db3f20f
Compare
github-actions
bot
added
size:M
Summary
Closes #490
This keeps active Codex turns projected as running when a non-terminal
runtime.errorarrives mid-turn.Previously, runtime ingestion treated any
runtime.erroras a terminal session error. In cases like Windowsos error 206(The filename or extension is too long), Codex could continue running in the background while the UI was projected as stopped/errored, leaving the session desynced and no longer stoppable from the UI.What Changed
runtime.errordoes not force the session intoerrorwhile the active turn is still runningturn.completedwith failed statesession.state.changedwitherrorsession.exitedruntime.errorwith no active turn still marks the session as erroredruntime.errorfor the active turn keeps the session running and records the runtime error activityWhy
This is a small reliability fix.
It keeps session state predictable under failure conditions and prevents the UI from desyncing from the actual Codex runtime when a mid-turn exec error is advisory rather than terminal.
Scope
This PR is intentionally narrow and only changes runtime session projection behavior for active-turn
runtime.errorevents.Testing
bun fmtbun lintbun typecheckbun x vitest run apps/server/src/orchestration/Layers/ProviderRuntimeIngestion.test.tsNotes
No UI change screenshots are included because this PR does not change visuals directly.
Note
Keep sessions running when
runtime.errorarrives during an active turnPreviously, any
runtime.errorevent would transition the session to an error state. Some providers emit advisory runtime errors while still streaming an active turn, so this change prevents premature session termination in that case.isActiveTurnRuntimeErrorflag detects when the error belongs to the currently active turn; the session is only set toerrorstatus when this flag is false.runningwithactiveTurnIdunchanged until a terminal lifecycle event arrives.runtime.errorevents tied to the active turn are now recorded in the thread but no longer end the session.📊 Macroscope summarized af1c65c. 5 files reviewed, 1 issue evaluated, 1 issue filtered, 0 comments posted
🗂️ Filtered Issues
apps/server/src/git/Layers/GitService.ts — 0 comments posted, 1 evaluated, 1 filtered
"output exceeded ${maxOutputBytes} bytes and was truncated."but this error is raised specifically whentruncateOutputisfalse— meaning the output was NOT truncated, an error was thrown instead. The message should say something like"output exceeded ${maxOutputBytes} bytes"without the misleading "and was truncated" clause. [ Posting failed ]